fix(website-listing): Avoid type error if item.description is not a string#12548
fix(website-listing): Avoid type error if item.description is not a string#12548
item.description is not a string#12548Conversation
|
Thanks for the PR, but I think this isn't a good idea. We don't to allow I'm sure there are other places. I concede that we should be documenting these and validating the YAML in listings. But I think description ought to always be a string. |
For a custom listing with a custom template is there any guarantee that you'll have any fields? I guess if so it'd be good to document the required fields. Anyway, this change isn't really arguing that you should allow description to be something else; more I'm suggesting that this is a place where the The error is super opaque, btw It'd be better to validate if (item.description !== undefined && !isPlaceHolder(item.description)) {
if (typeof item.description === "string" && !isPlaceHolder(item.description)) { |
|
I agree that we should be validating! But the validation should really be "item.description must be string", instead of "item.description might be |
Description
When using custom listings with a custom template, e.g.
template authors control how the contents are processed and may make odd decisions like including an item like this:
Their template code is handling the rendering, so they may process
item.descriptionconditionally and thenullis an okay value.However, Quarto's internal
isPlaceHolder(item.description)calls.match()on the description, which may not be a string, causing a type error.This PR fixes this by checking not just that
item.descriptionis defined but also that it's a string before callingisPlaceholder().(Sidenote: I'm not sure why this code path is triggered in the presence of a custom listing template, but I'm confident this would avoid the type error I encountered.)
Checklist
I have (if applicable):